-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
(WIP) Mtopo27/size analysis docs #15256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Bundle ReportChanges will increase total bundle size by 91.87kB (0.39%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-server-cjsAssets Changed:
view changes for bundle: sentry-docs-client-array-pushAssets Changed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I gave it a thorough read.
I think it will be more straightforward if we stick to our assumptions.
For example, if we say "assume the sentry-cli is installed" and then we say "apply the SENTRY_ACCESS_TOKEN" env variable, this will probably be a unnecessary step because they are already authed, but if they authed some other way, they might think the sentry-cli requires env variable auth token ONLY. So I just went and made sure if we are assuming something to not repeat the instructions and just link to other existing instructions.
I will give it another pass after this is applied so that I can see it visually in the deployment.
| ## Set your auth token | ||
|
|
||
| ```bash | ||
| export SENTRY_AUTH_TOKEN=your-token-here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are already assuming they have the sentry-cli installed, I will assume they have this already set up.
Otherwise, there are many ways to auth the sentry-cli. The easiest is probably running sentry-cli login so otherwise we should link to both installation and configuration of the sentry-cli and remove this section.
| export SENTRY_AUTH_TOKEN=your-token-here |
|
|
||
| ```groovy {filename:build.gradle} | ||
| plugins { | ||
| id "io.sentry.android.gradle" version "6.0.0-alpha.4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| id "io.sentry.android.gradle" version "6.0.0-alpha.4" | |
| id "io.sentry.android.gradle" version "6.0.0-alpha.5" |
armcknight
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that we should state an assumption for having sentry-cli installed or whatever else they need for it to work, like having environment variables set in CI.
To that point, it might make sense to split this into two sections: setup and local test, and then CI setup. It's a bit unclear why the second step talks about GitHub Actions env, and the the following step is to build the app locally.
It might flow better to do the setup and test it locally, then have a following section for setting it up for CI. This would have the benefit of getting them to first result faster in the first section.
| @@ -0,0 +1,17 @@ | |||
| ### Sentry CLI (Any platform) | |||
|
|
|||
| ```bash | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI in case you weren't aware, you can filter the content in includes by the current platform area being viewed, that way we don't display android instructions in Apple docs and vice-versa:
| ```bash | |
| <PlatformSection supported={["apple"]}> | |
| ```bash |
and same below for android. You'd have to split the code block into two different code blocks surrounded by these. I tried doing it all in one GH suggestion for you but the triple backticks are colliding so it's not possible and it doesn't render correctly here, but if you edit my comment you'll see the complete code, including the </PlatformSection>s:
<PlatformSection supported={["apple"]}>
export SENTRY_AUTH_TOKEN=your-token-here
# iOS
sentry-cli build upload app.xcarchive.zip \
--org your-org \
--project your-project \
--build-configuration Release<PlatformSection supported={["android"]}>
export SENTRY_AUTH_TOKEN=your-token-here
# Android
sentry-cli build upload app.aab \
--org your-org \
--project your-project \
--build-configuration freeReleaseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh we should do something similar for build.gradle vs build.gradle.kts
| ``` | ||
| ## Build your app | ||
| ## Building your app will trigger an upload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we should say that building an APK or bundle will trigger the upload on CI with the configuration.
| id("io.sentry.android.gradle") version "6.0.0-alpha.5" | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let’s just remove it from the build.gradle.kts here too since we removed it from the build.gradle.
docs/product/size-analysis/index.mdx
Outdated
|
|
||
| Set up the [GitHub integration](/product/size-analysis/github-integration/) to receive size change notifications as PR comments and status checks, making size considerations a natural part of your code review process. | ||
|
|
||
| ## Supported Platforms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we mention something about the hybrids (RN, Flutter) that they also work through the native uploaded bundles?
| To upload builds you'll need: | ||
|
|
||
| 1. A Sentry account with access to any [core plan](https://sentry.io/pricing/) | ||
| 2. A Sentry auth token – [generate one here](https://sentry.sentry.io/settings/auth-tokens/) and set `SENTRY_AUTH_TOKEN` in your environment so the tools can pick it up automatically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is our org, right? Shall we maybe embed a snippet where you can generate the auth token right in the docs, like here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point but I think we should just link to the docs on authenticating the CLI since using an environment variable might not be the preferred way to authenticate. (There are 4 other ways to provide the token).
coolguyzone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtopo27 Thanks, this was a big lift! Added several small suggestions, but this looks good to go. Once it passes the typo/404 checks it looks good to merge.
Co-authored-by: Alex Krawiec <[email protected]>
Update the minimum required version from 6.0.0-alpha.6 to 6.0.0-beta1 in the size analysis upload documentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…ntry/sentry-docs into docs/span-metrics-examples * 'docs/span-metrics-examples' of https://github.com/getsentry/sentry-docs: (27 commits) Adding logs callout in relevant breadcrumbs docs. (#15432) docs(self-hosted): Troubleshooting guide to invalidate projectconfigs (#15377) docs(js): Add metrics page (#15353) fix encoding for minification (#15429) update instructions for string minification (#15421) getsentry/relay@938582a fix: typos in TanStack Start docs (#15425) feat: Add firebase docs integration (#15310) feat(develop-docs): add index and subpages for Telemetry Buffer (#15424) docs(integration): Update Linear Agent Docs (#15420) feat(cloudflare): Fixed snippet to be cloudflare specific (#15369) chore(aws-lambda): Add troubleshooting section with known issue (#15414) getsentry/relay@5e23f1a docs(size-analysis): Align code boxes and remove vcsInfo links (#15418) feat(develop-docs): Add telemetry buffer page (#15411) Add Composer requirement to PHP platform guide (#15319) Updated link for version compatability (#15311) (WIP) Mtopo27/size analysis docs (#15256) Metrics Product Docs (#15375) ref(develop/spans): Replace `is_remote` with `is_segment` in Span Protocol (#15415) ...
DESCRIBE YOUR PR
adding docs for Size Analysis funcitonality
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes:
LEGAL BOILERPLATE
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
EXTRA RESOURCES